Skip to content

Some cleanups, fix memoryview support#122

Merged
pitrou merged 5 commits into
cloudpipe:masterfrom
pitrou:save_global_cleanup
Oct 27, 2017
Merged

Some cleanups, fix memoryview support#122
pitrou merged 5 commits into
cloudpipe:masterfrom
pitrou:save_global_cleanup

Conversation

@pitrou

@pitrou pitrou commented Oct 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

A couple assorted cleanups:

  • save_global can use the default implementation most of the time
  • save_buffer and save_memory needn't call Pickler.save_string directly
  • use save_reduce instead of dumping opcodes by hand where possible
  • remove Pandas from CI setup since it's not used anywhere in the test suite

Also, memoryviews should obviously serialize as bytes, not str.

@codecov-io

codecov-io commented Oct 26, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #122 into master will decrease coverage by 2.37%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
- Coverage   86.32%   83.94%   -2.38%     
==========================================
  Files           2        2              
  Lines         541      517      -24     
  Branches       98       90       -8     
==========================================
- Hits          467      434      -33     
- Misses         55       62       +7     
- Partials       19       21       +2
Impacted Files Coverage Δ
cloudpickle/cloudpickle.py 83.85% <90%> (-2.4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2da4c24...c91aaf1. Read the comment docs.

@pitrou

pitrou commented Oct 26, 2017

Copy link
Copy Markdown
Member Author

Predictably, Python 2.6 fails on memoryview. Should I fix this PR or should we ditch 2.6 (and 3.3)? :-)
@rgbkrk

@rgbkrk

rgbkrk commented Oct 26, 2017

Copy link
Copy Markdown
Member

How about we finish making the release for v0.4.1 since it's still Python 2.6 compatible, then we merge this in noting that we'll drop 2.6 and 3.3 support for v0.5.0

@pitrou

pitrou commented Oct 26, 2017

Copy link
Copy Markdown
Member Author

Works for me :-)

@rgbkrk

rgbkrk commented Oct 26, 2017

Copy link
Copy Markdown
Member

I'm going to give @ssanderson a day or so to make the release then I'll ping again and go from there. 😉

@hugovk

hugovk commented Oct 26, 2017

Copy link
Copy Markdown
Contributor

+1 to dropping 2.6 and 3.3.

Here's the pip installs for cloudpickle from PyPI for the last month -- they have very small numbers!

$ pypinfo --percent --pip cloudpickle pyversion
python_version percent download_count
-------------- ------- --------------
3.5              54.4%         41,113
2.7              30.5%         23,082
3.6              13.0%          9,834
3.4               1.9%          1,453
3.3               0.1%            103
3.7               0.1%             41
2.6               0.0%             12

@rgbkrk

rgbkrk commented Oct 26, 2017

Copy link
Copy Markdown
Member

We have more Python3 downloads than Python2 downloads?!?! 😮

@mrocklin

mrocklin commented Oct 26, 2017 via email

Copy link
Copy Markdown
Contributor

@rgbkrk

rgbkrk commented Oct 26, 2017

Copy link
Copy Markdown
Member

Alright, adjust that Travis matrix to get rid of 2.6 support and we can bring this in.

@pitrou pitrou merged commit 9bc424d into cloudpipe:master Oct 27, 2017
@pitrou pitrou deleted the save_global_cleanup branch October 27, 2017 10:27
This was referenced Oct 28, 2017
Closed
Closed
@ogrisel ogrisel mentioned this pull request Nov 13, 2017
Merged
@BryanCutler BryanCutler mentioned this pull request Jan 23, 2018
Closed
HyukjinKwon pushed a commit to HyukjinKwon/spark that referenced this pull request Mar 8, 2018
@BryanCutler @HyukjinKwon
[SPARK-23159][PYTHON] Update cloudpickle to v0.4.3
9bb239c 
## What changes were proposed in this pull request?

The version of cloudpickle in PySpark was close to version 0.4.0 with some additional backported fixes and some minor additions for Spark related things.  This update removes Spark related changes and matches cloudpickle [v0.4.3](https://github.com/cloudpipe/cloudpickle/releases/tag/v0.4.3):

Changes by updating to 0.4.3 include:
* Fix pickling of named tuples cloudpipe/cloudpickle#113
* Built in type constructors for PyPy compatibility [here](cloudpipe/cloudpickle@d84980c)
* Fix memoryview support cloudpipe/cloudpickle#122
* Improved compatibility with other cloudpickle versions cloudpipe/cloudpickle#128
* Several cleanups cloudpipe/cloudpickle#121 and [here](cloudpipe/cloudpickle@c91aaf1)
* [MRG] Regression on pickling classes from the __main__ module cloudpipe/cloudpickle#149
* BUG: Handle instance methods of builtin types cloudpipe/cloudpickle#154
* Fix <span>#</span>129 : do not silence RuntimeError in dump() cloudpipe/cloudpickle#153

## How was this patch tested?

Existing pyspark.tests using python 2.7.14, 3.5.2, 3.6.3

Author: Bryan Cutler <cutlerb@gmail.com>

Closes apache#20373 from BryanCutler/pyspark-update-cloudpickle-42-SPARK-23159.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pitrou @codecov-io @rgbkrk @hugovk @mrocklin